Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(extension): [lw-10782] add proper mappers for missed conway era certs #1234

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

vetalcore
Copy link
Contributor

@vetalcore vetalcore commented Jun 20, 2024

add proper mappers for missed conway era certs

Checklist


Proposed solution

Extend tx inspection related mappers with missed conway era certs.

Testing

Follow the steps in the description of the ticket.

Screenshots

Screenshot 2024-06-20 at 23 25 06 Screenshot 2024-06-20 at 23 25 16

@vetalcore vetalcore requested a review from mchappell June 20, 2024 20:31
@vetalcore vetalcore self-assigned this Jun 20, 2024
@vetalcore vetalcore requested a review from a team as a code owner June 20, 2024 20:31
Copy link

github-actions bot commented Jun 20, 2024

Allure Report

allure-report-publisher generated test report!

smokeTests: ✅ test report for 7e3cd8bc

passed failed skipped flaky total result
Total 33 0 0 0 33

Copy link
Collaborator

@mchappell mchappell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch 💪

Copy link
Contributor

@mirceahasegan mirceahasegan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @vetalcore ! 🥇

@vetalcore vetalcore force-pushed the fix/lw-10782-map-conway-era-certificates branch 2 times, most recently from dbed7ac to ff8f22d Compare June 24, 2024 20:48
@@ -22,6 +22,7 @@ export const useWalletActivities = ({

const fetchWalletActivities = useCallback(async () => {
fiatCurrency &&
cardanoFiatPrice &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a case when cardanoFiatPrice comes as undefined from useFetchCoinPrice
removing memoize util that is a hoc around mapWalletActivities also helps, but i'm not able to debug why resolver is not reevaluating the key correctly.

@@ -43,7 +43,7 @@ export const getFormattedFiatAmount = ({
return fiatAmount ? `${fiatAmount} ${fiatCurrency.code}` : '-';
};

const splitDelegationTx = (tx: TransformedActivity): TransformedTransactionActivity[] => {
const splitDelegationTx = (tx: TransformedActivity, hasConwayEraCerts: boolean): TransformedTransactionActivity[] => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could consider to refactor that at some point, maybe to hoist that into tx-inspection, so it could return an array of types.
@mchappell wdyt?
maybe we could create a tech debt ticket?

@vetalcore vetalcore force-pushed the fix/lw-10782-map-conway-era-certificates branch 4 times, most recently from b974066 to e2fadc0 Compare June 27, 2024 19:43
@vetalcore vetalcore force-pushed the fix/lw-10782-map-conway-era-certificates branch from 69ca3f4 to 7e3cd8b Compare July 1, 2024 06:26
Copy link

sonarqubecloud bot commented Jul 1, 2024

@vetalcore vetalcore merged commit 6222970 into main Jul 1, 2024
14 of 17 checks passed
@vetalcore vetalcore deleted the fix/lw-10782-map-conway-era-certificates branch July 1, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants